Add pre-commit and replace redundant CI lint steps by tigercosmos · Pull Request #714 · solvcon/modmesh

tigercosmos · 2026-04-05T17:31:00Z

Summary

Add .pre-commit-config.yaml with hooks matching CI lint checks:
- clang-format (v20.1.0) — C++ formatting for cpp/ and gtests/
- flake8 (7.3.0) — Python linting (excludes thirdparty/tmp/_deps)
- cinclude — Ensure C++ includes use angle brackets, not quotes
- checkascii — Ensure source files contain only ASCII characters
- checktws — Ensure no trailing whitespace in source files
Replace redundant CI lint steps with a single pre_commit job using pre-commit/action
Remove the clang_format_check job and individual make cinclude, make checkascii, make checktws, make flake8 steps from CI
Rename tidy_flake8 job to clang_tidy (now only runs clang-tidy + pilot builds)

Test plan

pre-commit run --all-files passes all 5 hooks locally
CI pre_commit job passes
CI clang_tidy job passes (unchanged functionality)

🤖 Generated with Claude Code

tigercosmos · 2026-04-05T17:32:39Z

contrib/prompt/CLAUDE.md

 make checkascii   # ASCII character check
 ```

+### Pre-commit


Let AI use pre-commit before it pushes the code.

tigercosmos · 2026-04-05T18:29:57Z

@yungyuc Please take a look. Thanks!

Add .pre-commit-config.yaml with hooks for clang-format, flake8, cinclude, checkascii, and checktws. Replace the redundant CI lint steps (clang_format_check job, cinclude, checkascii, checktws, flake8) with a single pre_commit job using pre-commit/action. Rename tidy_flake8 to clang_tidy since it now only runs clang-tidy and pilot builds. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

tigercosmos · 2026-04-05T19:15:15Z

.github/workflows/lint.yml

 jobs:

-  clang_format_check:
+  pre_commit:


Use pre-commit for CI to shorten the CI file.

yungyuc

I am not familiar with https://pre-commit.com/ . After skimming through the website, I do not see its benefits to modmesh.

For local hacking, I preferred to have all checks done manually. It is difficult to align the local development environments.

One who wants to run checks before committing can write a script, which looks like what $ pre-commit run --all-files does.

I am converting the PR to draft. We can discuss here.

yungyuc · 2026-04-06T11:48:39Z

.pre-commit-config.yaml

@@ -0,0 +1,50 @@
+# Copyright (c) 2026, An-Chi Liu <phy.tiger@gmail.com>


CI files do not need copyright.

yungyuc · 2026-04-06T11:48:41Z

.pre-commit-config.yaml

+# Copyright (c) 2026, An-Chi Liu <phy.tiger@gmail.com>
+# BSD-style license; see COPYING
+#
+# See https://pre-commit.com for more information


I am not sure an additional dependency for running the check is good.

ExplorerRay · 2026-04-06T12:18:40Z

As far as I know, pre-commit is a tool for developers to check some basic tests in local environments. After setting pre-commit up, it will check before people do git commit.

Although this tool adds some dependencies, these things are optional. The lint and code style we want to check still remain in the CI, so everything would be checked before merging.

Therefore, I think it's good because it can make developers do some basic prechecks in local side easier by packaging them under pre-commit.

This is the pre-commit docs I have read before from an open-source project called kepler. I think we may need a docs like this, otherwise if no one knows how to set up pre-commit, it might be meaningless to replace original step with pre-commit.

tigercosmos · 2026-04-06T13:08:04Z

pre-commit is useful for doing linting or styling checks. Of course we can manually run each check one by one, but it is more convinient to run by a single command (even better than a hand-crafted all-in-one script). As pre-commit is widely used by open source projects, it should be easy for developers to catch up and it can also benifit AI usage.

yungyuc · 2026-04-06T14:17:22Z

Feeling easy or not depends on familiarity. For someone not familiar with a tool, it will not be easy. From my personal perspective, the existing make lint is the easiest and the only dependency is make.

Could you please provide more objective analysis of why pre-commit is good (not necessarily better)? And where it is better than the existing make targets?

ExplorerRay · 2026-04-06T14:29:33Z

the existing make lint is the easiest and the only dependency is make.

I think current make lint also needs clang-format and flake8 as its dependencies.

Using pre-commit can make all those dependencies installed by pre-commit install and the versions of these dependencies can be controlled by .pre-commit-config.yaml.

tigercosmos · 2026-04-06T14:31:25Z

@ExplorerRay pointed out a good feature.
Other than that, one of my favoriate feature of pre-commit is that it can import actions (like GitHub Action) just like how a GitHub worflow import other pre-defined actions. With pre-commit, we can also introduce usuful predefined actions without our implementation, which can save a lot of time and gain benifits.

yungyuc · 2026-04-07T00:48:02Z

Using pre-commit can make all those dependencies installed by pre-commit install and the versions of these dependencies can be controlled by .pre-commit-config.yaml.

It is a plus to have an automatic way to manage the dependencies required by the actions. What are the limitations of the actions and action dependencies?

Other than that, one of my favoriate feature of pre-commit is that it can import actions (like GitHub Action) just like how a GitHub worflow import other pre-defined actions. With pre-commit, we can also introduce usuful predefined actions without our implementation, which can save a lot of time and gain benifits.

It sounds good to have additional modularity for building/CI. It probably worth of clarification for how it would strengthen or interfere the modularization that we want for the Github Actions. @tigercosmos @chestercheng @ExplorerRay I'd like to have your input on this.

the existing make lint is the easiest and the only dependency is make.

I think current make lint also needs clang-format and flake8 as its dependencies.

Good point. What I meant by "the only dependency" was for the tool itself, not the jobs it runs. Pre-commit has few dependencies but still more than make has.

tigercosmos commented Apr 5, 2026

View reviewed changes

tigercosmos force-pushed the worktree-pre-commit-setup branch from 59f56fc to 0ab7a60 Compare April 5, 2026 17:34

tigercosmos changed the title ~~Add pre-commit configuration for local lint checks~~ Add pre-commit and replace redundant CI lint steps Apr 5, 2026

tigercosmos force-pushed the worktree-pre-commit-setup branch from 91467d7 to 25433d1 Compare April 5, 2026 18:24

tigercosmos force-pushed the worktree-pre-commit-setup branch from 25433d1 to f0bf3ed Compare April 5, 2026 18:36

tigercosmos commented Apr 5, 2026

View reviewed changes

yungyuc requested review from ExplorerRay and chestercheng April 6, 2026 11:45

yungyuc requested changes Apr 6, 2026

View reviewed changes

yungyuc assigned tigercosmos Apr 6, 2026

yungyuc added refactor Change code without changing tests build Build system and automation labels Apr 6, 2026

yungyuc marked this pull request as draft April 6, 2026 11:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pre-commit and replace redundant CI lint steps#714

Add pre-commit and replace redundant CI lint steps#714
tigercosmos wants to merge 1 commit intosolvcon:masterfrom
tigercosmos:worktree-pre-commit-setup

tigercosmos commented Apr 5, 2026 •

edited

Loading

Uh oh!

tigercosmos Apr 5, 2026

Uh oh!

tigercosmos commented Apr 5, 2026

Uh oh!

tigercosmos Apr 5, 2026

Uh oh!

yungyuc left a comment

Uh oh!

yungyuc Apr 6, 2026

Uh oh!

yungyuc Apr 6, 2026

Uh oh!

ExplorerRay commented Apr 6, 2026

Uh oh!

tigercosmos commented Apr 6, 2026

Uh oh!

yungyuc commented Apr 6, 2026 •

edited

Loading

Uh oh!

ExplorerRay commented Apr 6, 2026

Uh oh!

tigercosmos commented Apr 6, 2026

Uh oh!

yungyuc commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,50 @@
		# Copyright (c) 2026, An-Chi Liu <phy.tiger@gmail.com>

Conversation

tigercosmos commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

tigercosmos Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

tigercosmos commented Apr 5, 2026

Uh oh!

tigercosmos Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

yungyuc left a comment

Choose a reason for hiding this comment

Uh oh!

yungyuc Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

yungyuc Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

ExplorerRay commented Apr 6, 2026

Uh oh!

tigercosmos commented Apr 6, 2026

Uh oh!

yungyuc commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ExplorerRay commented Apr 6, 2026

Uh oh!

tigercosmos commented Apr 6, 2026

Uh oh!

yungyuc commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tigercosmos commented Apr 5, 2026 •

edited

Loading

yungyuc commented Apr 6, 2026 •

edited

Loading